Makefile (1)

13-13: Consider alternatives to disabling pylint rules

Instead of disabling these rules globally, consider:

1. For R0913 (too-many-arguments):
   • Use data classes or test fixtures to group related parameters
   • Break down large test functions into smaller, focused ones
2. For R0917 (too-many-boolean-expressions):
   • Extract complex conditions into well-named helper methods
   • Use parameterized tests to reduce conditional complexity

Would you like assistance in implementing any of these suggestions?

lib/models/aerosurfaces.py (1)

6-13: Consider maintaining alphabetical class ordering

The Parachute class breaks the alphabetical ordering convention used in this file. Consider moving it after the NoseCone class to maintain consistency.

lib/controllers/rocket.py (1)

Line range hint 173-255: Consider improving error logging consistency.

The error logging messages use inconsistent method names:

• Log messages use controllers.rocket.update_rocket
• But the method name is update_rocket_by_id

Consider updating the log messages for consistency:

-            logger.error(f"controllers.rocket.update_rocket: PyMongoError {e}")
+            logger.error(f"controllers.rocket.update_rocket_by_id: PyMongoError {e}")
-            logger.error(f"controllers.rocket.update_rocket: {exc_str}")
+            logger.error(f"controllers.rocket.update_rocket_by_id: {exc_str}")
-                f"Call to controllers.rocket.update_rocket completed for Rocket {rocket_id}"
+                f"Call to controllers.rocket.update_rocket_by_id completed for Rocket {rocket_id}"

tests/test_routes/test_rockets_route.py (4)

36-47: Simplify fixture creation by using model_dump() instead of JSON serialization

In the stub_motor fixture, you're serializing the Motor object to JSON and then deserializing it back into a dictionary. You can simplify this process by using the model_dump() method to get a dictionary directly.

Apply this diff to simplify the fixture:

 def stub_motor():
     motor = Motor(
         thrust_source=[[0, 0]],
         burn_time=0,
         nozzle_radius=0,
         dry_mass=0,
         dry_inertia=[0, 0, 0],
         center_of_dry_mass_position=0,
     )
-    motor_json = motor.model_dump_json()
-    return json.loads(motor_json)
+    return motor.model_dump()

This approach reduces unnecessary serialization and improves the efficiency of your fixture.

---

51-62: Apply the same simplification to other fixtures

Similar to the stub_motor fixture, other fixtures like stub_tank, stub_level_tank, stub_mass_flow_tank, stub_ullage_tank, stub_mass_tank, stub_rocket_summary, stub_nose_cone, stub_fins, stub_tail, stub_rail_buttons, stub_parachute, and stub_rocket can be simplified by using model_dump().

For example, update the stub_tank fixture:

 def stub_tank():
     tank = MotorTank(
         geometry=[[(0, 0), 0]],
         gas=TankFluids(name='gas', density=0),
         liquid=TankFluids(name='liquid', density=0),
         flux_time=(0, 0),
         position=0,
         discretize=0,
         name='tank',
     )
-    tank_json = tank.model_dump_json()
-    return json.loads(tank_json)
+    return tank.model_dump()

Applying this change across all fixtures enhances code readability and reduces overhead.

Also applies to: 65-68, 71-84, 87-90, 93-98, 101-104, 108-119, 122-133, 136-147, 150-158, 161-172, 175-189

---

191-446: Use parameterization to reduce duplication in test cases

The test functions for creating rockets with different motor kinds and tank configurations have similar structures. You can utilize pytest.mark.parametrize to parameterize these tests, reducing code duplication and making the tests more maintainable.

Here's how you might refactor the tests:

@pytest.mark.parametrize(
    "motor_kind, stub_tank, motor_updates",
    [
        ("HYBRID", None, {
            'grain_number': 0,
            'grain_density': 0,
            'grain_outer_radius': 0,
            'grain_initial_inner_radius': 0,
            'grain_initial_height': 0,
            'grains_center_of_mass_position': 0,
            'grain_separation': 0,
            'throat_radius': 0,
        }),
        ("SOLID", None, {
            'grain_number': 0,
            'grain_density': 0,
            'grain_outer_radius': 0,
            'grain_initial_inner_radius': 0,
            'grain_initial_height': 0,
            'grains_center_of_mass_position': 0,
            'grain_separation': 0,
        }),
        ("LIQUID", stub_level_tank, {}),
        # Add other combinations as needed
    ],
)
def test_create_rocket_parametrized(
    stub_rocket, stub_motor, motor_kind, stub_tank, motor_updates
):
    if motor_updates:
        stub_motor.update(motor_updates)
    if stub_tank:
        stub_motor.update({'tanks': [stub_tank]})
    stub_rocket.update({'motor': stub_motor})
    with patch.object(
        RocketController,
        'create_rocket',
        return_value=RocketCreated(rocket_id='123'),
    ) as mock_create_rocket:
        with patch.object(
            Motor, 'set_motor_kind'
        ) as mock_set_motor_kind:
            response = client.post(
                '/rockets/', json=stub_rocket, params={'motor_kind': motor_kind}
            )
            assert response.status_code == 200
            assert response.json() == {
                'rocket_id': '123',
                'message': 'Rocket successfully created',
            }
            mock_set_motor_kind.assert_called_once_with(MotorKinds[motor_kind])
            mock_create_rocket.assert_called_once_with(Rocket(**stub_rocket))

This refactor enhances test coverage and simplifies maintenance.

---

197-200: Remove unnecessary side_effect=None when patching methods

When patching Motor.set_motor_kind, specifying side_effect=None is unnecessary because it defaults to None. You can simplify the code by omitting this parameter.

Apply this diff to each occurrence:

 with patch.object(
     Motor, 'set_motor_kind', side_effect=None
 ) as mock_set_motor_kind:
     # Test code

Simplify to:

 with patch.object(
     Motor, 'set_motor_kind'
 ) as mock_set_motor_kind:
     # Test code

This makes the code cleaner without affecting functionality.

Also applies to: 235-237, 265-267, 291-293, 316-318, 341-343, 366-368, 401-403, 434-436, 515-516

Configuration used: CodeRabbit UI
Review profile: CHILL

Learnt from: GabrielBarberini
PR: RocketPy-Team/Infinity-API#38
File: lib/routes/motor.py:75-75
Timestamp: 2024-11-15T15:12:21.314Z
Learning: When modifying API route paths in `lib/routes/motor.py`, changing from `/rocketpy/{motor_id}` to `/{motor_id}/rocketpy` is acceptable when no external consumers are affected. It's acceptable to introduce this breaking change if the team has decided to adopt the new approach, provided that the `README` and related documentation are updated accordingly.

#!/bin/bash
# Description: Find test files that trigger the disabled pylint rules

# Run pylint without disabling these rules to see which files are affected
pylint --disable=E0401,W0621 --enable=R0913,R0917 ./tests/ 2>&1 | grep -E "R0913|R0917"

#!/bin/bash
# Let's first check what test files exist
fd . tests/ -e py

# Then examine their content for complex boolean expressions and function signatures
for f in $(fd . tests/ -e py); do
  echo "=== $f ==="
  # Look for functions with many parameters
  ast-grep --pattern 'def $_($$$)' "$f" 2>/dev/null
  echo
  # Look for complex boolean expressions (multiple and/or operators)
  rg "(\s+and\s+|\s+or\s+).+(\s+and\s+|\s+or\s+)" "$f" 2>/dev/null
done

#!/bin/bash
# Let's examine the test files content directly to see function parameters and boolean expressions
echo "=== test_environments_route.py ==="
cat tests/test_routes/test_environments_route.py

echo -e "\n=== test_motors_route.py ==="
cat tests/test_routes/test_motors_route.py

echo -e "\n=== test_rockets_route.py ==="
cat tests/test_routes/test_rockets_route.py

#!/bin/bash
# Description: Verify Parachute class consistency across files

# Check if the Parachute import exists in rocket.py
echo "Checking Parachute import in rocket.py:"
rg "from .*.aerosurfaces import.*Parachute" lib/models/rocket.py

# Compare Parachute class definitions
echo -e "\nComparing Parachute class definitions:"
rg -A 7 "class Parachute" lib/models/rocket.py

#!/bin/bash
# Let's try a broader search to understand the usage and integration

# First, let's find all files that might reference Parachute
echo "Files containing Parachute references:"
rg -l "Parachute"

# Check for imports of aerosurfaces module
echo -e "\nImports of aerosurfaces module:"
rg "from.*aerosurfaces.*import"

# Look for actual usage of Parachute class
echo -e "\nUsage of Parachute class:"
rg -B 2 -A 2 "Parachute"

# Check if rocket.py exists and its content
echo -e "\nChecking rocket.py location:"
fd "rocket.py"

#!/bin/bash
# Let's examine the actual usage in lib/models/rocket.py and test files to verify field consistency

# Check the full Parachute class definition in aerosurfaces.py
echo "Parachute class definition in aerosurfaces.py:"
rg -A 10 "class Parachute" lib/models/aerosurfaces.py

# Check how Parachute is used in rocket.py
echo -e "\nParachute usage in lib/models/rocket.py:"
rg -B 5 -A 5 "parachutes.*List\[Parachute\]" lib/models/rocket.py

# Check test usage for field validation
echo -e "\nParachute test usage:"
rg -A 10 "stub_parachute\(\):" tests/test_routes/test_rockets_route.py

#!/bin/bash
# Description: Verify the signature of update_rocket_by_id in RocketController
ast-grep --pattern 'class RocketController:
  $$$
  async def update_rocket_by_id(rocket_id: str, rocket: $_) {
    $$$
  }
  $$$'

#!/bin/bash
# Let's try a different pattern to find the controller method
# First, let's locate the RocketController class file
fd "rocket.*controller" --type f

# Then let's use ripgrep to find the method definition with context
rg "async def update_rocket_by_id" -A 2

#!/bin/bash
# Search for all update methods in controllers to verify consistent parameter ordering
ast-grep --pattern 'async def update_$_by_id($$$)'

response = client.get('/rockets/123/rocketpy')
response = client.get('/motors/123/rocketpy')
response = client.get('/environments/123/rocketpy')

#!/bin/bash
# Search for any references to the old endpoint path pattern
rg -l "rocketpy/[^/]+" --type py

# Search for potential API client implementations
rg -l "flights/rocketpy/" --type py

#!/bin/bash
# Search for any references to the old endpoint pattern in Python files
rg "rocketpy/\{.*?\}" --type py

# Search for any references to the new endpoint pattern in Python files
rg "/\{.*?\}/rocketpy" --type py

# Search for any URLs or paths containing rocketpy
rg "rocketpy" --type py -C 2

return await RocketController.update_rocket_by_id(rocket_id, rocket)

async def update_rocket_by_id(self, rocket_id: str):

#!/bin/bash
# Search for calls to update_rocket_by_id to ensure they've been updated
rg -A 2 "update_rocket_by_id" --type py

#!/bin/bash
# Search for calls to update_flight_by_id to ensure they've been updated
rg -l "update_flight_by_id" | while read -r file; do
  if [[ "$file" != "lib/controllers/flight.py" ]]; then
    echo "Checking $file for update_flight_by_id calls:"
    rg -A 2 "update_flight_by_id" "$file"
  fi
done

#!/bin/bash
# Let's check the implementation of update_flight_by_id in the repository class
# to understand if it's a different method or needs parameter updates
ast-grep --pattern 'async def update_flight_by_id($$$)'

#!/bin/bash
# Let's check the implementation details with ripgrep instead
rg -A 5 "async def update_flight_by_id" lib/repositories/flight.py

#!/bin/bash
# Description: Search for usage of status code 203 in the codebase.

rg 'status_code\s*=\s*203' --type=py

#!/bin/bash
# Let's check the context around these 203 status codes to understand their usage pattern
rg 'status_code=203' -B 2 -A 2 --type=py

#!/bin/bash
# Let's check the specific endpoint in rocket.py to understand the full context
rg -B 10 -A 5 'status_code=203.*response_class=Response' lib/routes/rocket.py

#!/bin/bash
# Let's try a different approach to get the endpoint context
rg -B 10 -A 5 '@router\.get.*rocketpy' lib/routes/rocket.py

#!/bin/bash
# Let's check all endpoints that return binary data with 203 status code
rg -B 5 -A 5 'media_type="application/octet-stream"' lib/routes/

README.md (1)

177-177: LGTM! Consider adding deprecation notice.

The endpoint renaming from /rocketpy/model/:id to /model/:id/rocketpy follows better RESTful conventions by making the resource the primary path component. The sequence diagram accurately reflects this change and the data flow.

If the old endpoint /rocketpy/model/:id is still supported, consider adding a deprecation notice in the documentation to help users migrate to the new endpoint structure.

Configuration used: CodeRabbit UI
Review profile: CHILL

pyproject.toml (1)

Python 3.12 requirement seems unnecessary

The codebase uses match-case statements (introduced in Python 3.10) and basic f-strings, but no Python 3.12-specific features were found. The dependencies listed in requirements.txt are also compatible with Python 3.11. Consider relaxing the Python requirement to ">=3.11" to ensure broader adoption while maintaining modern Python features.

🔗 Analysis chain

Line range hint 3-4: Consider the impact of requiring Python 3.12

The Python requirement of ">=3.12" might be too restrictive as it's a very recent version (released Oct 2023). Consider if Python 3.11 compatibility could be maintained to ensure broader adoption and easier deployment.

Let's check the Python version requirements of dependencies:

---

Let me check the Python version requirements in pyproject.toml and examine FastAPI's version, as it's a major dependency that might influence the Python version requirement. This will help us understand if Python 3.12 is truly necessary or if we can support earlier versions.

---

Let me check if there are any Python 3.12-specific features being used in the codebase that would justify requiring such a recent version. This will help determine if we can safely support Python 3.11 as well.

---

Let me try searching for Python 3.12 specific features with simpler regex patterns to avoid parsing errors. This will help determine if there's any technical requirement for Python 3.12.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if dependencies support Python 3.12
# Get dependencies from requirements files if they exist
for req in requirements*.txt; do
  if [ -f "$req" ]; then
    echo "=== Checking $req ==="
    cat "$req"
  fi
done

Length of output: 587

---

Script:

#!/bin/bash
# Check RocketPy's Python version requirements
rg "python_requires|Python" pyproject.toml

# Check FastAPI's Python version requirements since it's a key dependency
rg "fastapi" requirements.txt -A 3 -B 3

Length of output: 324

---

Script:

#!/bin/bash
# Let's check if there are any Python 3.12-specific features being used
rg "match.*case:|type\s+.*\[.*\]|Self|TypeVarTuple|Unpack|assert\s+.*,\s+.*=|async\s+with\s+.*\s+as\s+.*,\s+.*:|f'{.*=.*}'" -t py

# Also check for any explicit version checks in the code
rg "sys\.version_info|python_version|requires.*3\.12" -g "!{*.txt,*.md,*.toml}"

Length of output: 531

---

Script:

#!/bin/bash
# Let's check for Python 3.12 specific features with separate searches
# Check for type annotations and match-case statements
rg "type\s+\w+\[" -t py
rg "match\s+.*:" -t py -A 2
rg "case\s+.*:" -t py

# Check for f-strings with = operator
rg "f['\"].*=.*['\"]" -t py

Length of output: 1806

lib/models/aerosurfaces.py (1)

11-11: Add validation for trigger string values

The trigger field accepts either altitude (float) or event name (str), but lacks validation for allowed event names.

+class TriggerEvent(str, Enum):
+    APOGEE = "apogee"
+    LAUNCH = "launch"
+    # Add other valid events...
+
 class Parachute(BaseModel):
     # ... other fields ...
-    trigger: Union[str, float]
+    trigger: Union[TriggerEvent, float]

Configuration used: CodeRabbit UI
Review profile: CHILL

#!/bin/bash
# Description: Check for potential code issues related to disabled rules

# Check for functions with many arguments
echo "=== Checking for functions with many parameters ==="
ast-grep --pattern 'def $FUNC($PARAMS) { $$$ }' | grep -E '\([^)]{80,}'

# Check for potential import issues
echo "=== Checking for potential import issues ==="
rg '^import' -A 1

#!/bin/bash
# Check for redefined outer names in test files (common case)
echo "=== Checking for potential redefined outer names in tests ==="
rg "^(\s+)?def test_.*:.*\n.*\s+\w+\s*=.*\n.*\s+\w+\s*=" -A 3

# Check for functions with many arguments in actual implementation
echo "=== Checking function definitions with parameters ==="
rg "^def \w+\([^)]+\)" -A 1

# Check for import errors by looking for relative imports and try-except around imports
echo "=== Checking for import error handling ==="
rg "^from \." -A 1
rg "try:.*\s+import" -A 2